-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Crossgen2 support for System.Private.CoreLib compilation in CoreCLR build #44090
Conversation
/cc @dotnet/crossgen-contrib Status update: the change kind of works to some extent with several caveats:
Thanks Tomas |
/cc @dotnet/runtime-infrastructure as this has tendrils into the build system. Minor changes required to CoreCLR build order so that Crossgen2 is built before crossgenning Corelib. If we decide for temporary parallel lab testing of CG1 & CG2, one possible option is to model it as two clr build steps - something like "clr.crossgencorelib" vs. "clr.crossgen2corelib". That would be easy for access from the YAML scripts while somewhat more annoying to use locally - one option would be the default (the default clr build step set per eng/Subsets.props) and triggering "the other version" via explicit specification of the build substeps would be cumbersome. The other possibility is to just propagate it around the system as a global property ("use CG1 vs. CG2") akin to Configuration, for instance; it would probably need to be settable as an extra argument to the root build script or via some environment variables as I tried to outline in the current formulation of the change. I'll welcome any feedback as to what option you see as the cleanest and most reasonable. |
Interestingly enough, out of the 11 failed legs in the build I claim my change only caused one, the CoreCLR release build on Windows ARM where R2RDump crashed on the GcInfo sync loss, the other ten leg failures are "someone else's problem" (which doesn't change the fact that my PR looks like it's crashing left and right in the lab). |
This initial change moves the actual SPC crossgenning logic from the cmd / sh scripts to the common proj script. I have yet to address cross-targeting and IBC, for now I'm striving for basic functionality. On Windows I'm currently hitting the absence of Microsoft.DiaSymReader.Native.amd64.dll that gets consumed via an ambient VS installation. Linking to the related issue. Thanks Tomas
As an expediency measure, I propose temporarily calling setup_vs_tools at the exact place we call crossgen to produce the System.Private.CoreLib PDB file. This highlights the local nature of the hotfix. Once we figure out a common strategy for VS installation interplay with runtime build on Windows, the hotfix can be easily removed. Thanks Tomas
The dotnet-runtime-perf failures are known and they are being currently investigated by @DrewScoggins. The last run ended up with a couple of failures in libraries / installer builds I believe to be unrelated to my change; I restarted them as I spotted some infrastructural hiccups in the failures. The failure in ARM Release build caused by R2RTest crash in GcInfo parsing is no longer happening after my latest change making GcInfo reading lazy. Overall I believe the change is ready to be reviewed and merged in. For now I haven't done anything about the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this change require any communication to "repo committers" so they are aware in case they see any issues with crossgen-ing corelib? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@mangod9 - that is an interesting question. My expectation is that not much should actually change; and we've been covering the crossgenning of Corelib in multiple lab tests for quite a while. But in general some form of heads-up that we're starting the transition to Crossgen2 would be probably useful. |
<CoreLibOutputPath>$(BinDir)\$(CoreLibAssemblyName).dll</CoreLibOutputPath> | ||
|
||
<CrossDir></CrossDir> | ||
<CrossDir Condition="'$(TargetArchitecture)' == 'arm' or '$(TargetArchitecture)' == 'arm64'">x64</CrossDir> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this change broke the native build on arm64 and arm because now when we try to run ./build.sh clr.nativecorelib -arch arm64 -c Debug -runtimeconfiguration Debug
we get:
Generating native image of System.Private.CoreLib for OSX.arm64.Debug. Logging to /Users/seandree/git/runtime/artifacts/log/CrossgenCoreLib_OSX__arm64__Debug.log
/Users/seandree/git/runtime/artifacts/bin/coreclr/OSX.arm64.Debug/x64/crossgen /nologo /Platform_Assemblies_Paths "/Users/seandree/git/runtime/artifacts/bin/coreclr/OSX.arm64.Debug/IL" /out "/Users/seandree/git/runtime/artifacts/bin/coreclr/OSX.arm64.Debug/System.Private.CoreLib.dll" "/Users/seandree/git/runtime/artifacts/bin/coreclr/OSX.arm64.Debug/IL/System.Private.CoreLib.dll"
/var/folders/wq/w3ybc8px5xgb0r5k7g6tkkdm0000gp/T/tmp7fd421fa5bce4bacb6316a02f0b68db9.exec.cmd: line 2: /Users/seandree/git/runtime/artifacts/bin/coreclr/OSX.arm64.Debug/x64/crossgen: No such file or directory
we should not use CrossDir
if HostArch
is the same as the target.
Fixes: #43810